-
Notifications
You must be signed in to change notification settings - Fork 25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: capability to send the client response to clipboard #130
Conversation
Thanks for your contribution! I will have a look into it! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like your solution! Maybe we could change the test a little bit.
cli/root_test.go
Outdated
defer wg.Done() | ||
var buf bytes.Buffer | ||
_, err := io.Copy(&buf, reader) | ||
_ = clipboard.WriteAll(buf.String()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we are overriding the values which were set in the root command. Therefore we are not actually testing the functionality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry you are right
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
cli/root_test.go
Outdated
root.Execute([]string{"--clipboard", prompt}) | ||
require.Equal(t, 0, mem.code) | ||
require.NoError(t, writer.Close()) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we could do something like this:
textInClipboard := clipboard.ReadAll() | |
require.Equals(t, expected, textInClipboard) |
With this we could remove the go func part, io.Pipe and writer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
The error in the CI pipeline is expected. The Github runner does not have a clipping tool installed. I will fix this test error after the merge. |
* feat: capability to send the client response to clipboard * feat: capability to send the client response to clipboard
Incredible tool, thank you for creating it, it has been very useful for me. I Created this simple pr to add the ability to save the client's response to the clipboard, it's not a big change, but it's something useful for me.